Skip to content

Make RequestFlags immutable with replace method#23

Merged
bisho merged 1 commit into
mainfrom
03-31-make_request_flags_frozen_to_avoid_side-effects
Apr 8, 2026
Merged

Make RequestFlags immutable with replace method#23
bisho merged 1 commit into
mainfrom
03-31-make_request_flags_frozen_to_avoid_side-effects

Conversation

@bisho
Copy link
Copy Markdown
Member

@bisho bisho commented Mar 31, 2026

Motivation / Description

The RequestFlags class was previously mutable, allowing fields to be modified after creation.

Routers and layers above meta-memcache-socket often share a single request flags object to avoid re-creating it, and we had a bug in the gutter pool where it was modifying the passed request flags that was shared across threads and requests.

This change makes RequestFlags immutable to improve safety and predictability, while providing a replace() method for conveniently creating modified copies.

Changes introduced

  • Made RequestFlags immutable by marking it as frozen and removing field setters
  • Added Final type annotations to all fields in the Python type stub
  • Replaced the copy() method with a new replace() method that accepts keyword arguments for selectively overriding fields
  • The replace() method preserves existing values for unspecified fields and handles optional fields appropriately
  • Bumped package version from 0.2.0 to 0.3.0
  • Added comprehensive tests for the new replace() functionality and immutability behavior

Copy link
Copy Markdown
Member Author

bisho commented Mar 31, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@bisho bisho requested review from a team and xmartinez March 31, 2026 10:35
@bisho bisho marked this pull request as ready for review March 31, 2026 10:35
Copy link
Copy Markdown

@xmartinez xmartinez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG!

nit: We should also update README.md (it references flags.copy()).

Comment thread src/request_flags.rs
///
/// Note: passing `None` explicitly for an optional field (e.g. `cache_ttl=None`)
/// keeps the existing value rather than unsetting it. To unset an optional field,
/// construct a new `RequestFlags` directly.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be confusing, but for common use cases (e.g., ttl override), it should be fine.

@bisho bisho force-pushed the 03-31-make_request_flags_frozen_to_avoid_side-effects branch from 9ac710e to 94522c5 Compare April 8, 2026 11:15
Copy link
Copy Markdown
Member Author

bisho commented Apr 8, 2026

Merge activity

  • Apr 8, 11:21 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Apr 8, 11:21 AM UTC: @bisho merged this pull request with Graphite.

@bisho bisho merged commit 1359ae3 into main Apr 8, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants